-
-
Notifications
You must be signed in to change notification settings - Fork 590
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix build failure in node.js example #4394
Fix build failure in node.js example #4394
Conversation
Relates to: element-hq/element-web#26922 Signed-off-by: Johannes Marbach <[email protected]>
examples/node/package.json
Outdated
"dependencies": { | ||
"cli-color": "^1.0.0", | ||
"matrix-js-sdk": "^32.0.0" | ||
"matrix-js-sdk": "^34.5.0-rc.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be nice to install the SDK from file:../..
instead and then do something on the CI to verify it doesn't regress again. I couldn't think of a good way to do that though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm torn on this. On the one hand, I'd like to set an example of how you might actually depend on the js-sdk in a project, which points towards an explicit version requirement here. On the other hand, I'd like to make sure that the examples continue to work as the js-sdk evolves, pointing towards file:./..
.
Maybe we can use an explicit version, but keep it up-to-date with Renovate?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds like a good compromise, yes. I'm slightly rusty on Renovate. It doesn't look like the example is currently picked up in the dashboard. I assume this is due to https://github.com/matrix-org/renovate-config-element-web/blob/main/default.json#L22. How exactly do we override this via the renovate.json
in this repository? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or actually, I guess we can just add this into https://docs.renovatebot.com/configuration-options/#filematch? Is there a way to test this without merging to develop
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have honestly no idea. Renovate is a mystery to me.
examples/node/package.json
Outdated
"dependencies": { | ||
"cli-color": "^1.0.0", | ||
"matrix-js-sdk": "^32.0.0" | ||
"matrix-js-sdk": "^34.5.0-rc.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm torn on this. On the one hand, I'd like to set an example of how you might actually depend on the js-sdk in a project, which points towards an explicit version requirement here. On the other hand, I'd like to make sure that the examples continue to work as the js-sdk evolves, pointing towards file:./..
.
Maybe we can use an explicit version, but keep it up-to-date with Renovate?
Co-authored-by: Richard van der Hoff <[email protected]>
Co-authored-by: Richard van der Hoff <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM for now at least
c8403f3
Relates to: #4287
Relates to: element-hq/element-web#26922
Signed-off-by: Johannes Marbach [email protected]
Checklist
public
/exported
symbols have accurate TSDoc documentation.